[3.0] Misc fixes - #9302

Okay, I think I am done messing with this PR.

@sbulen, would you care to test whether it solves the problem of not being logged after the upgrader exits?

2.1 => 3.0

Fatal error: Uncaught Error: Cannot access uninitialized non-nullable property SMF\User::$username by reference in D:\wamp64\www\van2130\Sources\User.php:807 Stack trace: # 0 [internal function]: SMF\User->$member_name::get() # 1 D:\wamp64\www\van2130\Sources\Utils.php(1539): json_encode(Array, 0, 512) # 2 D:\wamp64\www\van2130\Sources\Services\ErrorHandlerService.php(286): SMF\Utils::jsonEncode(Array) # 3 D:\wamp64\www\van2130\Sources\Services\ErrorHandlerService.php(185): SMF\Services\ErrorHandlerService->log('Cannot access u...', 'general', 'D:/wamp64/www/v...', 807, Array) # 4 D:\wamp64\www\van2130\Sources\ErrorHandler.php(104): SMF\Services\ErrorHandlerService->catch(Object(Error)) # 5 [internal function]: SMF\ErrorHandler::catch(Object(Error)) # 6 {main} thrown in D:\wamp64\www\van2130\Sources\User.php on line 807

• PHP, MySQL, Apache logs: nothing
• Upgrader log: nothing

Well, phooey. I must have broken something in my tinkering after I first pushed. I suppose that's what I get for trying to code while sleepy.

@sbulen:

1. I've made changes to this PR. Have they fixed the error about uninitialized properties?
2. If so, does this PR solve the problem of not being logged after the upgrader exits?

2.1 => 3.0

Getting a different WSOD:

Fatal error: Uncaught Error: Call to a member function allowedTo() on array in D:\wamp64\www\van2130\Sources\Maintenance\Maintenance.php:661 Stack trace: 0 D:\wamp64\www\van2130\Sources\Maintenance\Tools\Upgrade.php(819): SMF\Maintenance\Maintenance::loginAdmin('admin_shawnb', Object(SensitiveParameterValue), false) 1 D:\wamp64\www\van2130\Sources\Maintenance\Maintenance.php(294): SMF\Maintenance\Tools\Upgrade->welcomeLogin() 2 D:\wamp64\www\van2130\upgrade.php(24): SMF\Maintenance\Maintenance->execute(2) 3 {main} thrown in D:\wamp64\www\van2130\Sources\Maintenance\Maintenance.php on line 661

PHP error log reflects the above. MySQL & Apache logs clear.

Upgrade.log:

2026-08-08T17:11:08+00:00
Step 1: Login
Making sure SMF can write to "D:\wamp64\www\van2130\Settings.php"... done.
Making sure SMF can write to "D:\wamp64\www\van2130\Settings_bak.php"... done.
Making sure SMF can write to "D:\wamp64\www\van2130/custom_avatar"... done.
Making sure SMF can write to "D:/wamp64/www/van2130/cache/db_last_error.php"... done.

That should be fixed now. The cause was just a silly mistake on my part. If you could test again, @sbulen, that would be lovely!

Note this branch currently has merge conflicts.

Fixed now, @sbulen. Ready for a retest whenever you are able.

I went over this while working on upgrade hardening, because the DropTimeOffset fix in here is a prerequisite for a 2.1 → 3.0 upgrade completing at all on release-3.0. Reporting what I ran and three things I noticed, none of which I think block it.

What I verified

Merged into current release-3.0 (clean, no conflicts), then:

The browser run is the one that mattered, since that is what this PR is mostly about: login → upgrade options → 245 migration substeps → 72 utf8mb4 substeps → 2 cleanup substeps → finalize → redirect to index.php, with no PHP notices, warnings or deprecations anywhere in the rendered output. The dynamic security token renders and validates, so the Utils::$context switch is doing its job. Point 2 of the description checks out as well — the row that lands is id_member=1, admin, {"version":"SMF 3.0 Alpha 4","member_acted":"admin"}.

I also chased three things that looked like problems and turned out not to be, in case they save anyone else the trip: the guest dataset dropping from Basic to Minimal does not affect login, because Minimal selects mem.* and so still has passwd, password_salt and passwd_flood; the CacheApi::$enable default plus max() is a fix, since displayDbError() raises it before calling CacheApi::get() and reading the uninitialised typed Config::$cache_enable would otherwise throw inside the error handler; and dropping $last_slash from Install.php keeps the CLI fix, just inline via str_contains().

1. A change to password handling that is not in the description

This is the only one I would want a second opinion on before it merges.

In the old Login2::checkPasswordFallbacks(), the SMF-native fallbacks used the decoded password but the other forum software ones used the raw POST value:

// SMF 1.1 / 2.0 — decoded
$other_passwords[] = sha1(strtolower($this->member->username) . Utils::htmlspecialcharsDecode($_POST['passwrd']));

// phpBB3, vBulletin, MyBB, PunBB, … — raw
$other_passwords[] = md5($_POST['passwrd']);
$other_passwords[] = crypt($_POST['passwrd'], substr($_POST['passwrd'], 0, 2));
$other_passwords[] = hash('sha256', $_POST['passwrd']);
$other_passwords[] = $_POST['passwrd'];

Security::checkPasswordFallbacks() now uses $password throughout, and Login2 passes Utils::htmlspecialcharsDecode($_POST['passwrd']). So every converted-forum fallback now sees the decoded value where it previously saw the encoded one.

I think the new behaviour is the correct one — the source software hashed the literal password, so & in a password should be & and not &. But it changes whether a converted user whose password contains & < > " ' can log in, for anyone who has not logged in since the conversion, and it is an authentication change that the description does not mention. Worth either a line in the description or a deliberate confirmation that it is intended.

2. AttachmentDirectory loses its guard

isCandidate() is removed, so the migration now runs on every pass. The rewritten execute() is idempotent, so that is safe in itself. Two smaller things about it:

• When none of the four formats parse, it silently falls back to [1 => Config::$boarddir . '/attachments'], discarding whatever the admin had. For a value that is merely unusual rather than invalid, that points attachments somewhere new without saying so. A logged warning on that branch would make it recoverable.
• $current_attach_dir = Config::$modSettings['currentAttachmentUploadDir'] ?? array_key_first($attach_dirs) keeps an existing value even when it names a key that is not in $attach_dirs — which is exactly the case after the fallback above.

3. Placeholder docblock

RemoveTempAttachments::error() carries undocumented method. The two language strings it reaches for both exist with the right placeholders (attach_directory_admin_warning takes {attach_dir}, cant_access_upload_path takes {path}), so only the docblock needs a moment.

1. A change to password handling that is not in the description

This is the only one I would want a second opinion on before it merges.

In the old Login2::checkPasswordFallbacks(), the SMF-native fallbacks used the decoded password but the other forum software ones used the raw POST value:

// SMF 1.1 / 2.0 — decoded
$other_passwords[] = sha1(strtolower($this->member->username) . Utils::htmlspecialcharsDecode($_POST['passwrd']));

// phpBB3, vBulletin, MyBB, PunBB, … — raw
$other_passwords[] = md5($_POST['passwrd']);
$other_passwords[] = crypt($_POST['passwrd'], substr($_POST['passwrd'], 0, 2));
$other_passwords[] = hash('sha256', $_POST['passwrd']);
$other_passwords[] = $_POST['passwrd'];

Security::checkPasswordFallbacks() now uses $password throughout, and Login2 passes Utils::htmlspecialcharsDecode($_POST['passwrd']). So every converted-forum fallback now sees the decoded value where it previously saw the encoded one.

I think the new behaviour is the correct one — the source software hashed the literal password, so & in a password should be & and not &. But it changes whether a converted user whose password contains & < > " ' can log in, for anyone who has not logged in since the conversion, and it is an authentication change that the description does not mention. Worth either a line in the description or a deliberate confirmation that it is intended.

That was an unintentional change, although I do agree that the new behaviour is correct. However, purely for the sake of backward compatibility, I will add some code to try variants of htmlspecialchars($password) within the loop that checks for passwords from other forum software.

2. AttachmentDirectory loses its guard

isCandidate() is removed, so the migration now runs on every pass. The rewritten execute() is idempotent, so that is safe in itself. Two smaller things about it:

• When none of the four formats parse, it silently falls back to [1 => Config::$boarddir . '/attachments'], discarding whatever the admin had. For a value that is merely unusual rather than invalid, that points attachments somewhere new without saying so. A logged warning on that branch would make it recoverable.
• $current_attach_dir = Config::$modSettings['currentAttachmentUploadDir'] ?? array_key_first($attach_dirs) keeps an existing value even when it names a key that is not in $attach_dirs — which is exactly the case after the fallback above.

• Those four formats are the only ones considered valid.
• Good catch. Fixed now.